Skip to content

fix(api-websockets): deliver server->client pushes on SQL deployments (recency filter) - #5553

Open
adrians5j wants to merge 7 commits into
nextfrom
adrian/self-hosted-watch-message
Open

fix(api-websockets): deliver server->client pushes on SQL deployments (recency filter)#5553
adrians5j wants to merge 7 commits into
nextfrom
adrian/self-hosted-watch-message

Conversation

@adrians5j

@adrians5j adrians5j commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

On SQL-backed (self-hosted / server flavour) deployments, SendToIdentity silently matched zero connections, so server→client WebSocket pushes never arrived — even though the connection was registered under the correct identity. Symptom: file-manager AI image enrichment completes and updates the file, but no "Image enriched" notification appears in Admin.

Root cause

ListConnectionsUseCase filters out stale connections with a lexicographic string comparison:

const threeHoursAgo = new Date(Date.now() - 3 * 60 * 60 * 1000).toISOString();
connections = connections.filter(c => c.connectedOn >= threeHoursAgo);

connectedOn is declared string and is written as a UTC ISO string, but it is stored in a SQL datetime column. On read, the injected knex/driver hands it back in a shape that does not compare against an ISO string. On the reporter's SQLite setup it came back as a Date object (coerces to "Wed Aug 04 2026 …", which sorts below "2026-…"); other drivers/configs return a T/Z-less "2026-08-04 17:02:06" (space 0x20 sorts before T 0x54). Either way every live connection reads as older than the cutoff and is dropped → SendToIdentity sends to nobody, throws nothing, task still reports success.

Fix in this PR (unblocks the symptom)

Compare by parsed UTC epoch instead of raw string, handling both Date and string forms (normalizing the space form back to UTC — safe, since the stored wall-clock is UTC). Extracted into connectedOnToEpoch / isRecentConnection helpers + a named window constant.

⚠️ This is a localized patch — two deeper issues remain (please read)

This PR fixes the one call site so pushes work again, but it papers over the actual root cause. Two follow-ups worth doing properly:

A. connectedOn type contract is violated (blast radius beyond this filter)

connectedOn is typed string everywhere (ConnectionRegistry abstractions, ConnectionRow), but at runtime it's whatever the injected knex returns for a datetime column (a Date on the reporter's setup). The SQL adapter's toData() passes the raw driver value straight through, so the lie propagates to every consumer — notably WebsocketsGraphQLFactory exposes connectedOn: DateTime! in the Admin GraphQL, where a Date vs an ISO string can serialize inconsistently.

  • Proper fix: normalize connectedOn/lastSeen to canonical ISO once, at the SQL boundary (in WebsocketsConnectionRegistry.toData() in @webiny/api-websockets-sql). Then the string type is honest, GraphQL is consistent, and the recency comparison could even go back to a plain string compare. The framework receives an injected knex it doesn't control, so normalizing at this boundary is the correct layer.

B. Recency filter keys off the wrong field (engine-independent correctness bug)

ListConnectionsUseCase judges "recent" by connectedOn (first-connect time), but listStale (in the SQL registry) correctly judges staleness by lastSeen (the heartbeat timestamp). So an Admin session open > 3 hours — alive and actively heartbeating, lastSeen fresh — has an old connectedOn and gets filtered out, silently losing all pushes. The heartbeat/lastSeen machinery exists precisely for liveness; the send path ignores it.

  • Proper fix: filter on lastSeen (fall back to connectedOn when never heard from). Independent of the date-format issue above.

Recommended end state

Do A (boundary normalization in toData()) + B (filter on lastSeen), then the epoch helper in this PR can be simplified or removed. Happy to redo the PR that way if preferred over the current patch.

Not a WS-wiring regression

The recent 6.5.0 merges did not revert any WebSocket source — the wiring (register → sql registry → SendToIdentity → transport → admin handler) is intact. This is a latent bug in a new code path. On AWS/DynamoDB it never bites: ddb stores connectedOn as a plain string attribute that round-trips as ISO, so the original string compare is valid there.

Verification

Reproduced end-to-end on a self-hosted SQL project: upload image → AI enrichment task → SendToIdentity now matches the live connections → "Image enriched" notification renders in Admin. Traced with temporary logging that showed the sql registry returning matched=N while SendToIdentity saw matchedConnections=0, isolating the drop to this recency filter.

🤖 Generated with Claude Code

adrians5j and others added 7 commits August 4, 2026 12:56
Watching all apps at once isn't supported yet for the self-hosted hosting
type, but the post-create message told users to run a single `webiny watch`
command. Update the message to run the API and Admin apps separately.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…placeholder

oxfmt reformatted the admin index.tsx `{GLOBAL_CSS}` placeholder into a block
statement, so the literal find-replace in ServerBuildAppWorkspaceService no
longer matched and the bare `GLOBAL_CSS;` shipped to the workspace (TS2304).
Exclude the server template's appTemplates folder from oxfmt (mirroring the
AWS template) and restore the placeholder.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…config

The 6.5.0 stale-branch merge (#5396) clobbered createRsbuildConfig.js back to a
pre-#5453 state, dropping the `assetPrefix: "auto"` config and the isServer
externals gating. Without assetPrefix, the self-hosted bg-tasks worker chunk
(spawned via `new Worker(new URL(..., import.meta.url))`) resolved to an
absolute filesystem-root URL and failed with "Cannot find module". The
referencing comments in WorkerTaskService/BreeSchedulerService survived the
revert, masking the loss. Restore the #5453 version.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SendToIdentity silently matched zero connections on SQL-backed (self-hosted)
deployments, so server->client pushes (e.g. the file-manager AI enrichment
notification) never arrived even though the connection was registered under the
correct identity.

ListConnectionsUseCase filtered stale connections with a lexicographic string
comparison (`connectedOn >= <iso cutoff>`). But `connectedOn` is a SQL
`datetime` column, and the driver returns it in a shape that doesn't compare
against an ISO string: mysql2 hands back a `Date` (coerces to "Wed Aug 04
2026 ...", sorts below "2026-...") and other drivers return a `T`/`Z`-less
"2026-08-04 17:02:06" (space sorts before `T`). Either way every live
connection read as expired and was dropped.

Compare by parsed UTC epoch instead, normalizing the space form back to UTC
(safe — the stored wall-clock is UTC), so it works regardless of driver format.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the connectedOn epoch parsing and recency predicate out of execute() into
module-level helpers (connectedOnToEpoch, isRecentConnection) plus a named
RECENT_CONNECTION_WINDOW_MS constant. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant